Skip to content

BIP352: complete type annotations on bitcoin_utils class methods#2160

Merged
jonatack merged 1 commit into
bitcoin:masterfrom
omipheo:bip-0352-class-method-annotations
Jun 1, 2026
Merged

BIP352: complete type annotations on bitcoin_utils class methods#2160
jonatack merged 1 commit into
bitcoin:masterfrom
omipheo:bip-0352-class-method-annotations

Conversation

@omipheo
Copy link
Copy Markdown
Contributor

@omipheo omipheo commented May 15, 2026

Summary

Direct follow-up to PR #2154 (which annotated the free-function half of bip-0352/bitcoin_utils.py) and 2f7117c ("BIP352: fix Any typing"). The four classes in this file — COutPoint, VinInfo, CScriptWitness, and CTxInWitness — still had unannotated
__init__, serialize, deserialize, and is_null methods, so mypy could not flow types through the surrounding reference.py
code that constructs and passes these objects.

This PR annotates every method on all four classes:

Class Methods annotated
COutPoint __init__(hash: bytes, n: int) -> None, serialize() -> bytes, deserialize(f: BytesIO) -> None
VinInfo __init__ (typed Optional defaults for the three construct-on-None fields)
CScriptWitness __init__() -> None, is_null() -> bool, plus an inline stack: List[bytes] declaration matching what the rest of the file already assumes
CTxInWitness __init__() -> None, deserialize(f: BytesIO) -> "CTxInWitness" (forward ref since it returns self), is_null() -> bool

Optional is added to the existing typing import; List was already added by #2154. No behavior changes.

Verification

$ mypy --ignore-missing-imports ./bip-0352/bitcoin_utils.py
Success: no issues found in 1 source file

$ python3 -c "
import sys
sys.path.insert(0, 'bip-0352')
sys.path.insert(0, 'bip-0352/secp256k1lab/src')
import bitcoin_utils
from io import BytesIO

op1 = bitcoin_utils.COutPoint(b'\\x11' * 32, 7)
data = op1.serialize()
print('COutPoint serialize len:', len(data))
op2 = bitcoin_utils.COutPoint()
op2.deserialize(BytesIO(data))
print('COutPoint round-trip:', op2.hash.hex()[:16], op2.n)

csw = bitcoin_utils.CScriptWitness()
print('CScriptWitness empty is_null:', csw.is_null())
csw.stack.append(b'\\x01\\x02')
print('CScriptWitness with-stack is_null:', csw.is_null())

ctw = bitcoin_utils.CTxInWitness()
print('CTxInWitness deserialize returned self:',
      ctw.deserialize(BytesIO(b'\\x00')) is ctw)

vi = bitcoin_utils.VinInfo()
print('VinInfo defaults:', type(vi.outpoint).__name__,
      type(vi.txinwitness).__name__, type(vi.private_key).__name__)
"
COutPoint serialize len: 36
COutPoint round-trip: 1111111111111111 7
CScriptWitness empty is_null: True
CScriptWitness with-stack is_null: False
CTxInWitness deserialize returned self: True
VinInfo defaults: COutPoint CTxInWitness Scalar

$ cd bip-0352 && python3 -c "import reference" (exits 0 — reference.py still imports cleanly)

Notes

Direct follow-up to PR bitcoin#2154 (which annotated the free-function half
of bip-0352/bitcoin_utils.py) and 2f7117c ("BIP352: fix Any typing").
The four classes in this file — COutPoint, VinInfo, CScriptWitness,
and CTxInWitness — still had unannotated `__init__`, `serialize`,
`deserialize`, and `is_null` methods. mypy could not flow types
through the surrounding reference.py code that constructs and passes
these objects.

Annotate every method on all four classes:

- COutPoint:    __init__ (hash, n), serialize -> bytes, deserialize -> None
- VinInfo:      __init__ (typed Optional defaults for the three
                construct-on-None fields)
- CScriptWitness: __init__ -> None, is_null -> bool, plus an inline
                stack: List[bytes] declaration matching what the
                rest of the file already assumes
- CTxInWitness: __init__ -> None, deserialize -> "CTxInWitness"
                (forward ref since it returns self), is_null -> bool

Adds Optional to the existing typing import (List was already added
by bitcoin#2154). No behavior changes.

Verified: mypy --ignore-missing-imports ./bip-0352/bitcoin_utils.py
reports "Success: no issues found in 1 source file"; round-trip
smoke tests on COutPoint serialize/deserialize, CScriptWitness
is_null with empty + non-empty stack, CTxInWitness deserialize
returning self, and VinInfo default-construction all match the
pre-change behavior.
@murchandamus
Copy link
Copy Markdown
Member

Personally, I’m not sure whether the juice is worth the squeeze, but ymmv. @theStack, please let me know if you want these changes, otherwise, please feel free to let me know to close this.

Copy link
Copy Markdown
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review and tested ACK 1259a23

@jonatack jonatack added the Fixups Minor fixups not worth bothering the BIP author(s) for label Jun 1, 2026
@jonatack jonatack merged commit d58e12a into bitcoin:master Jun 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fixups Minor fixups not worth bothering the BIP author(s) for

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants